-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add control-plane token expiry #620
Conversation
@@ -9,10 +9,13 @@ import ( | |||
"github.com/spf13/cobra" | |||
) | |||
|
|||
const minTokenTTL = 3 * time.Second |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, so you can't make a token that is valid for 2 seconds but 4 seconds is fine? Am i reading that right... no complaints just making sure i understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I added this because we also do this for the timeout
flag. My assumption was that this is done to avoid error if the user assumes a unit (e.g. seconds) and does not set them in the flag. However, it seems like this would just fail:
➜ sudo k8s get-join-token test --expires-in 10
Error: invalid argument "10" for "--expires-in" flag: time: missing unit in duration "10"
Usage:
k8s get-join-token <node-name> [flags]
Flags:
--expires-in duration the time until the token expires (default 24h0m0s)
-h, --help help for get-join-token
--timeout duration the max time to wait for the command to execute (default 1m30s)
--worker generate a join token for a worker node
So, I don't see a reason to actually have this check - removed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @bschimke95! Just a minor comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks a lot!
Microcluster v2 supports token TTL. This PR exposes this configuration option to the API/CLI.
Note that is only applicable for the control-plane tokens right now. Workers will be done in separate work (since we don't use microcluster for that it is more involved as we need to implement the logic ourselves).
This PR requires the (backward-compatible) update in the
k8s-snap-api
.canonical/k8s-snap-api#5
Once this PR is reviewed and approved, I will merge the API change, create a new tag (
1.0.3
) and update thisk8s-snap
PR to use the new API version.